Skip to content

fix[next-dace]: Fix SDFG lowering of neighbors-reduce on field with non-zero origin#2569

Open
edopao wants to merge 8 commits intoGridTools:mainfrom
edopao:compile_time_domain_enable_warning
Open

fix[next-dace]: Fix SDFG lowering of neighbors-reduce on field with non-zero origin#2569
edopao wants to merge 8 commits intoGridTools:mainfrom
edopao:compile_time_domain_enable_warning

Conversation

@edopao
Copy link
Copy Markdown
Contributor

@edopao edopao commented Apr 9, 2026 •
edited
Loading

This PR re-enables a warning which was hit in ICON4Py by the dace backend, because we forced the backend to always compute fields everywhere, as a workaround for an issue in the dace backend, also when gt4py can infer the minimal domain at compile time.

The issue in the dace backed was caused by a bug in de-referencing a field computed on the minimal domain. Tested in icon4py PR C2SM/icon4py#1190

@edopao edopao changed the title fix[next]: Eenable warning for max domain on compile-time domain fix[next]: Enable warning for max domain on field operators with compile-time domain Apr 9, 2026
@edopao edopao mentioned this pull request Apr 15, 2026
Open
@edopao edopao requested a review from tehrengruber April 15, 2026 12:03
tehrengruber
tehrengruber reviewed Apr 16, 2026
View reviewed changes
Comment thread src/gt4py/next/iterator/transforms/pass_manager.py Outdated
tehrengruber
tehrengruber approved these changes Apr 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@tehrengruber tehrengruber left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an integration test? test_unstructured_shift called with a field of non-zero domain start / origin should trigger this too right? Maybe we should also rename the PR since the main part is the fix not really the reintroduction of the warning.

@edopao
Copy link
Copy Markdown
Contributor Author

edopao commented Apr 16, 2026

Can we add an integration test? test_unstructured_shift called with a field of non-zero domain start / origin should trigger this too right? Maybe we should also rename the PR since the main part is the fix not really the reintroduction of the warning.

Good point. I have added two test cases. One is a variant of test_unstructured_shift, as you suggested, however this case was already working. The other one is testing neighbor_sum with a field of non-zero origin, and this was failing. Anyway, both are relevant: the first covers the lowering of unstructured shift operator and the second reduce+neighbors builtins.

@edopao edopao changed the title fix[next]: Enable warning for max domain on field operators with compile-time domain fix[next-dace]: Fix SDFG lowering of neighbors-reduce on field with non-zero origin Apr 16, 2026
@edopao edopao mentioned this pull request May 6, 2026
Draft
edopao
edopao commented May 7, 2026
View reviewed changes
Comment thread tests/next_tests/integration_tests/feature_tests/ffront_tests/test_execution.py
Comment on lines +753 to +754
if unstructured_case.backend is None:
pytest.xfail("Embedded backend requires contiguous inverse image.")
Copy link
Copy Markdown
Contributor Author

@edopao edopao May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@havogt Would that be OK to merge the tests with this xfail?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tehrengruber tehrengruber tehrengruber approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@edopao @tehrengruber